@adhiamboperes @BenHenning this is a part of #5343 (but a standalone fix), can you PTAL.

Unassigning @Rd4dev since a re-review was requested. @Rd4dev, please make sure you have addressed all review comments. Thanks!

APK & AAB differences analysis

Note that this is a summarized snapshot. See the CI artifacts for detailed differences.

Dev

Alpha

Beta

Ga

Thanks @Rd4dev! This looks good to me.

Seperately, do you have insights on why the non-app module workflows take long to cancel?

Thanks @adhiamboperes,

Reg Non-app module cancellation delays, I haven't really dug into why it takes a lot of time. However, based on observation, it predominantly occurs during the Non-app modules' Build App step, which seems to wait for certain tasks to finish before the cancellation trigger is hit or recognized.

I may try to trigger the cancellation once the Build App step is complete to see if other stages cancel immediately without delay (which I believe is the case based on a previous observation, but I will confirm with another set of runs to identify all causes).

APK & AAB differences analysis

Note that this is a summarized snapshot. See the CI artifacts for detailed differences.

Dev

Alpha

Beta

Ga

@adhiamboperes, based on the observation it seems that every job step that uses the expression if: ${{ always() }} exhibit the delay in their cancellation.

Since, the expression always

Causes the step to always execute, and returns true, even when canceled.

Example case:

with Static Checks workflow we have:

jobs:
  linters:
    name: Lint Tests
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/checkout@v2

      .............

      - name: Java lint check
        run: |
          bash /home/runner/work/oppia-android/oppia-android/scripts/checkstyle_lint_check.sh $HOME

      - name: Kotlin lint check
        run: |
          bash /home/runner/work/oppia-android/oppia-android/scripts/ktlint_lint_check.sh $HOME

      - name: Feature flag checks
        run: |
          bash /home/runner/work/oppia-android/oppia-android/scripts/feature_flags_check.sh $HOME

      .............

  script_checks:
    name: Script Checks
    runs-on: ubuntu-20.04
    env:
      CACHE_DIRECTORY: ~/.bazel_cache
    steps:
      - uses: actions/checkout@v2

      .............

      - name: Regex Patterns Validation Check
        if: always()
        run: |
          bazel run //scripts:regex_pattern_validation_check -- $(pwd)

      - name: XML Syntax Validation Check
        if: always()
        run: |
          bazel run //scripts:xml_syntax_check -- $(pwd)

      - name: Testfile Presence Check
        if: always()
        run: |
          bazel run //scripts:test_file_check -- $(pwd)

      - name: Accessibility label Check
        if: always()
        run: |
          bazel run //scripts:accessibility_label_check -- $(pwd) scripts/assets/accessibility_label_exemptions.pb app/src/main/AndroidManifest.xml

      - name: KDoc Validation Check
        if: always()
        run: |
          bazel run //scripts:kdoc_validity_check -- $(pwd) scripts/assets/kdoc_validity_exemptions.pb

      .............

Screenshot:

[Stacktrace]

The Lint Tests steps didn't have the expression if: ${{ always() }} causing it to stop immediately when cancelled.

With the Script Checks steps having the if: ${{ always() }} expression set, makes the following steps to execute even after getting cancelled. And only after a certain time they get hit by the cancel trigger and actually stop their execution.

I will investigate further to see if there could be any workaround to stop them immediately and update them.

Edit:

Workaround

Seems like there is a workaround and as per docs, it is suggested to use the expression if: ${{ !cancelled() }}, using this allows it to still run a job or step regardless of its success or failure while allowing it to get cancelled. So, I replaced all the if: ${{ always() }} with if: ${{ !cancelled() }} and also confirmed that this doesn't interrupt the job still being called even if it failed (with the unit test workflow).

Warning: Avoid using always for any task that could suffer from a critical failure, for example: getting sources, otherwise the workflow may hang until it times out. If you want to run a job or step regardless of its success or failure, use the recommended alternative: if: ${{ !cancelled() }}

[Related Discussion]

Thanks for highlighting the issue, with this now the workflows get cancelled immediately when a new run is triggered.

but I'm also curious about one thing: per your PR description, it doesn't actually seem like https://github.com/styfle/cancel-workflow-action is being used or am I missing something?

Uhm.. Sorry if the description was unclear. And I am not sure which one was mentioned here.

If that was the PR description mentioning - "The existing workflow canceller utilized the cancel-workflow-action by syfle."

By that I was mentioning that the currently existing workflow_canceller.yml workflow file is using the styfle/cancel-workflow-action (to just reference the existing cancellation method) which is now being replaced with GitHub's concurrency. Am I pointing the same or have I mentioned it incorrectly elsewhere?

Thanks for the review, @BenHenning. I've added the suggested in-line comments to each instance, can you PTAL.

Unassigning @Rd4dev since a re-review was requested. @Rd4dev, please make sure you have addressed all review comments. Thanks!

APK & AAB differences analysis

Note that this is a summarized snapshot. See the CI artifacts for detailed differences.

Dev

Alpha

Beta

Ga

Going ahead and merging this since it shouldn't have upstream dependencies, and it'd be nice to immediately begin benefiting from the fix. :)